fix(contributor-profile): add GitHub response validation#708
fix(contributor-profile): add GitHub response validation#708Tanayajadhav1 wants to merge 1 commit into
Conversation
✅ Deploy Preview for github-spy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughContributorProfile and Contributors pages now validate GitHub API responses before state updates and surface structured error states. Type guards ensure data shape correctness; distinct error handling branches address 404, rate-limit, and timeout scenarios. Error UI displays status-specific messaging. ChangesAPI Error Handling and Validation Pattern
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Improves reliability and UX of GitHub data fetching on the Contributors list and Contributor Profile pages by adding runtime response validation and more detailed error handling.
Changes:
- Added structured error state and expanded error UI for contributors fetch failures (rate limit, 404, timeout, invalid payload).
- Added runtime type guards and
fetch()response status handling for contributor profile + PR search. - Introduced early-return error UI for missing username / request failures.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| src/pages/Contributors/Contributors.tsx | Adds runtime validation + richer error object/state and more detailed error UI for contributor fetches. |
| src/pages/ContributorProfile/ContributorProfile.tsx | Adds runtime validation, better HTTP status handling, and an error state/UI for profile + PR fetching. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <Typography variant="caption" sx={{ display: "block", mt: 1 }}> | ||
| You've hit GitHub's API rate limit. The limit resets in 1 hour. | ||
| </Typography> |
|
hi @mehul-m-prajapati please review this PR whenever possible! |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/pages/ContributorProfile/ContributorProfile.tsx (2)
112-115:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winHandle clipboard write failures (promise rejection).
navigator.clipboard.writeText(...)returns a Promise, buthandleCopyLinkdoesn’tawaitor catch it, so clipboard/permission errors won’t trigger any user-facing error state.Suggested fix
- const handleCopyLink = () => { - navigator.clipboard.writeText(window.location.href); - toast.success("🔗 Shareable link copied to clipboard!"); - }; + const handleCopyLink = async () => { + try { + await navigator.clipboard.writeText(window.location.href); + toast.success("🔗 Shareable link copied to clipboard!"); + } catch { + toast.error("Failed to copy link. Please copy it manually."); + } + };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/ContributorProfile/ContributorProfile.tsx` around lines 112 - 115, The handleCopyLink function currently calls navigator.clipboard.writeText(window.location.href) without awaiting or handling rejections; update handleCopyLink to await the writeText Promise (or use .then/.catch) and on success call toast.success("🔗 Shareable link copied to clipboard!"), and on failure call toast.error with a user-friendly message and optionally log the error; target the handleCopyLink function and the navigator.clipboard.writeText call and ensure clipboard permission/exception paths are handled.
48-110:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCancel in-flight GitHub requests on
usernamechanges to prevent stale state updates.
useEffectstarts asyncfetchcalls for the user and PRs but never aborts them; whenusernamechanges quickly, an older response can still callsetProfile/setPRs/setError. Add anAbortController, passcontroller.signalto bothfetchcalls, abort in theuseEffectcleanup, and ignoreAbortError.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/ContributorProfile/ContributorProfile.tsx` around lines 48 - 110, The effect's async fetchData starts GitHub fetches that can resolve after username changes and cause stale setProfile/setPRs/setError updates; update fetchData/useEffect to create an AbortController, pass controller.signal into both fetch calls (the user fetch and prs fetch), abort the controller in the useEffect cleanup, and in the catch block ignore errors where err.name === 'AbortError' so you don't call setState/toast for canceled requests.src/pages/Contributors/Contributors.tsx (1)
224-225:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd
rel="noopener noreferrer"for external new-tab links.Opening contributor GitHub pages with
target="_blank"withoutrelexposes reverse-tabnabbing risk.Suggested fix
<Button variant="contained" startIcon={<FaGithub />} href={contributor.html_url} target="_blank" + rel="noopener noreferrer" sx={{🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/pages/Contributors/Contributors.tsx` around lines 224 - 225, In the anchor/link elements in Contributors.tsx that set target="_blank" (the JSX elements around the contributor GitHub links where you currently have target="_blank" and sx={{...}}), add rel="noopener noreferrer" to the same element to prevent reverse-tabnabbing; update each instance of the anchor/Link with target="_blank" to include rel="noopener noreferrer".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/pages/ContributorProfile/ContributorProfile.tsx`:
- Line 60: The code interpolates username raw into GitHub URLs (e.g., the fetch
call in ContributorProfile.tsx: const userRes = await
fetch(`https://api.github.com/users/${username}``) and the PR search q
parameter), which can break requests for usernames with special characters; fix
by applying encodeURIComponent(username) wherever username is inserted into URLs
(both the user fetch and the PR search query construction) so the resulting
URL/query is properly escaped while preserving existing logic and variable
names.
- Around line 17-24: The runtime type checks in ContributorProfile use `as any`;
replace those with safe narrowing via `Record<string, unknown>` and a small
`hasItems` helper: in `isProfile`, cast `data` to `const obj = data as
Record<string, unknown>` and check properties with `typeof obj.avatar_url ===
'string'`, `typeof obj.login === 'string'`, and `(typeof obj.bio === 'string' ||
obj.bio === null)`. Add a `function hasItems(value: unknown): value is unknown[]
{ return Array.isArray(value) && value.length > 0; }` and use it where array
casts were done to validate non-empty arrays and element types instead of `as
any`. Apply the same pattern to the other runtime guards in this file (replace
all `as any` usages) so all runtime parsing is done via `Record<string,
unknown>` checks and `hasItems` assertions.
---
Outside diff comments:
In `@src/pages/ContributorProfile/ContributorProfile.tsx`:
- Around line 112-115: The handleCopyLink function currently calls
navigator.clipboard.writeText(window.location.href) without awaiting or handling
rejections; update handleCopyLink to await the writeText Promise (or use
.then/.catch) and on success call toast.success("🔗 Shareable link copied to
clipboard!"), and on failure call toast.error with a user-friendly message and
optionally log the error; target the handleCopyLink function and the
navigator.clipboard.writeText call and ensure clipboard permission/exception
paths are handled.
- Around line 48-110: The effect's async fetchData starts GitHub fetches that
can resolve after username changes and cause stale setProfile/setPRs/setError
updates; update fetchData/useEffect to create an AbortController, pass
controller.signal into both fetch calls (the user fetch and prs fetch), abort
the controller in the useEffect cleanup, and in the catch block ignore errors
where err.name === 'AbortError' so you don't call setState/toast for canceled
requests.
In `@src/pages/Contributors/Contributors.tsx`:
- Around line 224-225: In the anchor/link elements in Contributors.tsx that set
target="_blank" (the JSX elements around the contributor GitHub links where you
currently have target="_blank" and sx={{...}}), add rel="noopener noreferrer" to
the same element to prevent reverse-tabnabbing; update each instance of the
anchor/Link with target="_blank" to include rel="noopener noreferrer".
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d303be58-4f2b-4172-8f75-c56d6415d814
📒 Files selected for processing (2)
src/pages/ContributorProfile/ContributorProfile.tsxsrc/pages/Contributors/Contributors.tsx
| const isProfile = (data: unknown): data is Profile => { | ||
| return ( | ||
| typeof data === "object" && | ||
| data !== null && | ||
| typeof (data as any).avatar_url === "string" && | ||
| typeof (data as any).login === "string" && | ||
| (typeof (data as any).bio === "string" || (data as any).bio === null) | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify remaining explicit-any usage in this file
rg -n --type=ts --type=tsx '\bas\s+any\b' src/pages/ContributorProfile/ContributorProfile.tsxRepository: GitMetricsLab/github_tracker
Length of output: 100
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/pages/ContributorProfile/ContributorProfile.tsx"
echo "== any/as any occurrences =="
rg -n --no-heading --hidden --follow --glob "$FILE" '\bas\s+any\b|\bno-explicit-any\b|\b:\s*any\b' "$FILE" || true
echo
echo "== context around the previously cited lines (15-45) =="
nl -ba "$FILE" | sed -n '1,120p'
echo
echo "== context around the previously cited lines (70-120) =="
nl -ba "$FILE" | sed -n '70,140p'Repository: GitMetricsLab/github_tracker
Length of output: 719
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/pages/ContributorProfile/ContributorProfile.tsx"
wc -l "$FILE"
echo
echo "== Lines 1-60 =="
cat -n "$FILE" | sed -n '1,60p'
echo
echo "== Lines 70-120 =="
cat -n "$FILE" | sed -n '70,120p'Repository: GitMetricsLab/github_tracker
Length of output: 3996
Remove as any casts in runtime guards/PR parsing to satisfy no-explicit-any and keep validation type-safe.
src/pages/ContributorProfile/ContributorProfile.tsx still uses as any at lines 21-23, 34-36, 91-92, and 97. Replace these casts with Record<string, unknown>-based narrowing and a small hasItems guard.
Suggested fix
+type UnknownRecord = Record<string, unknown>;
+
const isProfile = (data: unknown): data is Profile => {
+ const d = data as UnknownRecord;
return (
typeof data === "object" &&
data !== null &&
- typeof (data as any).avatar_url === "string" &&
- typeof (data as any).login === "string" &&
- (typeof (data as any).bio === "string" || (data as any).bio === null)
+ typeof d.avatar_url === "string" &&
+ typeof d.login === "string" &&
+ (typeof d.bio === "string" || d.bio === null)
);
};
const isPrArray = (data: unknown): data is PR[] => {
return (
Array.isArray(data) &&
data.every(
(item) =>
typeof item === "object" &&
item !== null &&
- typeof (item as any).title === "string" &&
- typeof (item as any).html_url === "string" &&
- typeof (item as any).repository_url === "string"
+ typeof (item as UnknownRecord).title === "string" &&
+ typeof (item as UnknownRecord).html_url === "string" &&
+ typeof (item as UnknownRecord).repository_url === "string"
)
);
};
+
+const hasItems = (data: unknown): data is { items: unknown } =>
+ typeof data === "object" && data !== null && "items" in data;
- if (
- typeof prsData !== "object" ||
- prsData === null ||
- !Array.isArray((prsData as any).items) ||
- !isPrArray((prsData as any).items)
- ) {
+ if (!hasItems(prsData) || !Array.isArray(prsData.items) || !isPrArray(prsData.items)) {
throw new Error("Invalid GitHub PR response.");
}
- setPRs((prsData as any).items);
+ setPRs(prsData.items);🧰 Tools
🪛 ESLint
[error] 21-21: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 22-22: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 23-23: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
[error] 23-23: Unexpected any. Specify a different type.
(@typescript-eslint/no-explicit-any)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/ContributorProfile/ContributorProfile.tsx` around lines 17 - 24,
The runtime type checks in ContributorProfile use `as any`; replace those with
safe narrowing via `Record<string, unknown>` and a small `hasItems` helper: in
`isProfile`, cast `data` to `const obj = data as Record<string, unknown>` and
check properties with `typeof obj.avatar_url === 'string'`, `typeof obj.login
=== 'string'`, and `(typeof obj.bio === 'string' || obj.bio === null)`. Add a
`function hasItems(value: unknown): value is unknown[] { return
Array.isArray(value) && value.length > 0; }` and use it where array casts were
done to validate non-empty arrays and element types instead of `as any`. Apply
the same pattern to the other runtime guards in this file (replace all `as any`
usages) so all runtime parsing is done via `Record<string, unknown>` checks and
`hasItems` assertions.
| setError(null); | ||
|
|
||
| try { | ||
| const userRes = await fetch(`https://api.github.com/users/${username}`); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and print the relevant section around the cited line(s).
FILE="src/pages/ContributorProfile/ContributorProfile.tsx"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE"
exit 1
fi
echo "== File: $FILE =="
wc -l "$FILE"
sed -n '1,140p' "$FILE" | nl -ba | sed -n '40,100p'
# Also search for usages of `username` and GitHub URL construction within the file.
echo "== rg: username in ContributorProfile.tsx =="
rg -n "username" "$FILE"
echo "== rg: github.com in ContributorProfile.tsx =="
rg -n "github\.com" "$FILE"Repository: GitMetricsLab/github_tracker
Length of output: 254
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/pages/ContributorProfile/ContributorProfile.tsx"
echo "== File: $FILE =="
wc -l "$FILE"
echo "== Lines 40-100 =="
awk 'NR>=40 && NR<=100 {printf "%d\t%s\n", NR, $0}' "$FILE"
echo "== rg: username in ContributorProfile.tsx =="
rg -n "username" "$FILE" || true
echo "== rg: github.com in ContributorProfile.tsx =="
rg -n "github\.com" "$FILE" || true
echo "== Print fetch/search URL constructions =="
rg -n "fetch\\(|search/issues" "$FILE" || trueRepository: GitMetricsLab/github_tracker
Length of output: 3230
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/pages/ContributorProfile/ContributorProfile.tsx"
rg -n "encodeURIComponent" "$FILE" || trueRepository: GitMetricsLab/github_tracker
Length of output: 54
Encode username before building GitHub URLs.
username is interpolated raw into the GitHub user fetch URL and the PR search q parameter; special characters can break requests or change query semantics.
Suggested fix
- const userRes = await fetch(`https://api.github.com/users/${username}`);
+ const encodedUsername = encodeURIComponent(username);
+ const userRes = await fetch(`https://api.github.com/users/${encodedUsername}`);
...
- `https://api.github.com/search/issues?q=author:${username}+type:pr`
+ `https://api.github.com/search/issues?q=author:${encodedUsername}+type:pr`🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/ContributorProfile/ContributorProfile.tsx` at line 60, The code
interpolates username raw into GitHub URLs (e.g., the fetch call in
ContributorProfile.tsx: const userRes = await
fetch(`https://api.github.com/users/${username}``) and the PR search q
parameter), which can break requests for usernames with special characters; fix
by applying encodeURIComponent(username) wherever username is inserted into URLs
(both the user fetch and the PR search query construction) so the resulting
URL/query is properly escaped while preserving existing logic and variable
names.
Related Issue
Description
This PR adds runtime validation and error handling to the ContributorProfile page when fetching GitHub user and pull request data.
Changes made:
res.ok.How Has This Been Tested?
Screenshots (if applicable)
N/A
Type of Change
suggested labels:
gssoc'26, level:intermediate, bug, type:fix , size:s , area:quality,priority:medium
Summary by CodeRabbit